-
-
Notifications
You must be signed in to change notification settings - Fork 125
Glasgow | ITP-May-2025 | Nataliia Volkova | Sprint 1 | Destructuring #232
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall this is very good. You are making great progress. There are some stylistic issues to address.
|
||
const result = gryffindorMembers(hogwarts); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The functions are good. However there are a couple of issues with this line. What would happen if you didn't pass in hogwarts
as a parameter? What is the value of the result
variable? Also, there's a spacing issue - Javascript convention is to have no space between the function name and the opening parenthesis for parameters.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my case, the answer will be the same, because I use hogwarts
inside my function. To use hogwarts
as a parameter, I would write code differently to pass, for example, an array
in parentheses as a parameter in declaring a function and use the array
to filter members. The type value of the result variable is an object(be clear, an array).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello Andrew, In my case, the answer will be the same, because I use hogwarts
inside my function. To use hogwarts
as a parameter, I would write code differently to pass, for example, an array
in parentheses as a parameter in declaring a function and use thearray
to filter members. The type value of the result variable is an object(be clear, an array).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes the hogwarts
variable was declared globally so there was no need to pass it into the function. The function doesn't return anything so result
will be undefined
.
function getTotalOfOrder (order) { | ||
let Total = 0; | ||
console.log("QTY".padEnd(8), "ITEM".padEnd(24), "TOTAL"); | ||
order.forEach(({itemName, quantity, unitPricePence}) => { | ||
const QTY = quantity; | ||
const ITEM = itemName; | ||
const TOTAL = ((unitPricePence * quantity)/100); | ||
Total = Total + TOTAL; | ||
console.log(`${String(QTY).padEnd(6)} ${String(ITEM).padEnd(24)} ${(TOTAL.toFixed(2)).padStart(4)}`); | ||
}) | ||
|
||
console.log(`\nTotal: ${Total.toFixed(2)}`) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The destructuring implementation follows best practices and the core functionality works correctly, producing the expected output. However, there are a few areas that need attention. Please ensure all variable and function names use camelCase consistently throughout the code. Additionally, consider whether the intermediate variables are really required or if some could be removed. The indentation also needs a little correction. I'd suggest running the code through prettier or some other code formatter to address styling issues automatically.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for focusing my attention on intermediate variables. I removed variables that I used only once and those whose absence would not influence code readability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your new implementation is really good.
Learners, PR Template
Self checklist
Changelist
Briefly explain your PR.
Questions
Ask any questions you have for your reviewer.